stdarch-test: various cleanups
#1860
Conversation
| let found = expected == "nop" | ||
| || instrs.iter().any(|instruction| { | ||
| instruction.starts_with(expected) | ||
| // Check that the next character is non-alphabetic. This prevents false negatives |
There was a problem hiding this comment.
I would expect digits (is_ascii_digit) to also be part of the instruction name.
There was a problem hiding this comment.
That is what I had earlier, but it gave lots of hits for ld1 where the test just asserts on ld
There was a problem hiding this comment.
I don't see any intrinsics that specifically match on "ld". All the load instrinsics explicitly specify the number since this is an important part of the instruction.
There was a problem hiding this comment.
hmm maybe it was a different instruction? In any case it gave a lot of false positives somewhere.
There was a problem hiding this comment.
I would prefer if this included digits and had a way to explicitly opt-in to wildcard matching.
| // Check that the next character is non-alphabetic. This prevents false negatives | ||
| // when e.g. `fminnm` was used but `fmin` was expected. | ||
| // | ||
| // TODO: resolve the conflicts (x86_64 has a bunch, probably others) |
There was a problem hiding this comment.
Are you intending to have this merged as it is, or wait for x86 conflicts to be resolved first? I'm happy to merge either way.
There was a problem hiding this comment.
This is good to go, we can work through the false positives over time (it's not the most fun task).
folkertdev
force-pushed
the
stdarch-test-cleanup
branch
from
77ccca1 to
ded4a23
Compare
My goal here was to make the instruction matching stricter, to prevent accepting e.g.
fminnmwhen the test specifies thefmininstruction must be used.But especially on x86 there are lots of cases where a prefix is (deliberately?) used.
Anyway, I also made some other cleanups. Can be reviewed commit-by-commit.